Fixes #28065 - package dynflow sidekiq service files #4194

ezr-ondrej · 2019-10-15T20:47:04Z

Depend on:

Package dependencies Add rubygem-sidekiq{,-reliable-fetch} #4188
Prepare init script Fixes #28064 - initial script for sidekiq foreman#7109

ekohl

This is also missing the macros to install the service. I now see we do this under %preun and %postun but that's actually incorrect for subpackages. We should fix this for the -service subpackage as well. d7b23d1 (with 7f9c972) is a good example of how a subpackage can introduce a service.

packages/foreman/foreman/dynflow@.service

packages/foreman/foreman/foreman.spec

ehelms · 2019-10-18T00:55:36Z

packages/foreman/foreman/foreman.spec

@@ -711,6 +713,34 @@ Meta Package to install requirements for Redis caching support
 %files redis
 %{_datadir}/%{name}/bundler.d/redis.rb

+%package dynflow-sidekiq


Does this package conflict with the regular dynflow service? Can both run at the same time? What happens if they do? Is the new dynflow-sidekiq optional?

No, it doesn't conflict, and it is not trying to remove it, if someone runs both, they will share the load and loadbalance.
dynflow-sidekiq is installed in installer only if the option is turned on (theforeman/puppet-foreman#761) so yes, it's optional.

Does the singleton nature of the dynflow sidekiq orchestrator play well with the current dynflow executor if both are deployed simultaneously?

Yeah, that doesn't really matter, the separation is not really affecting the old deployment.

packages/foreman/foreman/foreman.spec

ekohl · 2019-10-22T14:15:27Z

packages/foreman/foreman/foreman.spec

+sed -i '/^ExecStart/ s|/usr/bin/sidekiq \(.\+\)$|/usr/bin/scl enable tfm "sidekiq \1"|' %{buildroot}%{_unitdir}/%{dynflow_sidekiq_service_name}.service
+
+for i in orchestrator.yml worker.yml; do
+mv %{buildroot}%{_datadir}/%{name}/config/dynflow/$i %{buildroot}%{_sysconfdir}/%{name}/dynflow/


Can you indent these lines?

ekohl · 2019-10-22T14:20:33Z

packages/foreman/foreman/foreman.spec

+%{_unitdir}/%{dynflow_sidekiq_service_name}.service
+%{_datadir}/%{name}/script/dynflow-sidekiq.rb
+
+%install dynflow-sidekiq


I don't think you can have separate %install sections. At least, we don't use it anywhere else.

So I should install it in the main package directly? or to use build section?

Main %install is fine. Then within %files you split it up again.

ekohl · 2019-10-22T15:00:18Z

Also needs an entry in comps.

ezr-ondrej · 2019-10-22T15:33:04Z

Added comps section, moved install to main install.

ekohl

The build is failing because you need to increase the release and add a changelog.

ezr-ondrej · 2019-10-22T15:43:48Z

Add changelog line

ekohl · 2019-10-22T16:00:35Z

The build failure is due to the nodejs changes that are going on.

ehelms · 2019-10-22T17:18:55Z

This current design installs and starts "classic" dynflow by default. As I understood things, the target was for the new dynflow-sidekiq to be the default?

If we are keeping "classic" dynflow as the default, installing this new subpackage will configure and start dynflow-sidekiq while leaving classic dynflow running as is. Is this what a user should expect? That feels like it will lead to inconsistencies.

ezr-ondrej · 2019-10-22T17:41:14Z

@ehelms this is now addressed in the installer PR theforeman/puppet-foreman#761 it will disable the old service for you once you select the sidekiq to be installed. If you are not using installer you're on your own to figure it out though.

tbrisker · 2019-10-23T17:22:51Z

this is now blocked on the core PR: BUILDSTDERR: install: cannot stat 'extras/systemd/dynflow-sidekiq@.service': No such file or directory

tbrisker · 2019-10-24T07:14:50Z

[test rpm]

ezr-ondrej · 2019-10-24T09:40:37Z

Any idea why the tests are failing now? 😞

ekohl · 2019-10-24T09:47:17Z

Because there wasn't a new source tarball with the new file. There is now.
[test rpm]

ezr-ondrej · 2019-10-24T10:03:32Z

🍾 Thanks @ekohl.

ekohl

I'd like @ehelms to ack as well before merging.

ekohl · 2019-10-24T10:18:37Z

packages/foreman/foreman/foreman.spec

@@ -2,6 +2,7 @@
 %global confdir extras/packaging/rpm/sources
 %global foreman_rake %{_sbindir}/%{name}-rake
 %global executor_service_name dynflowd
+%global dynflow_sidekiq_service_name dynflow-sidekiq@


Given the repetition I'm inclined to add .service here but I'm fine either way. I'd probably do a cleanup of all service handling later anyway.

I would like to stay consistent with the dynflowd service, otherwise I would agree 🤔

ehelms · 2019-10-24T14:11:56Z

Figured I would give this a basic runtime test, I am seeing the following (note the service timed out starting):

Oct 24 14:05:58 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:05:58.249Z 14667 TID-6hbxf INFO: GitLab reliable fetch activated!
Oct 24 14:05:58 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:05:58.251Z 14667 TID-fnj6v INFO: Booting Sidekiq 5.2.7 with redis options {:id=>"Sidekiq-server-PID-14667", :url=>nil}
Oct 24 14:06:00 centos7-katello-nightly.war.example.com scl[14647]: /usr/share/foreman/lib/foreman.rb:8: warning: already initialized constant Foreman::UUID_REGEXP
Oct 24 14:06:00 centos7-katello-nightly.war.example.com scl[14647]: /usr/share/foreman/lib/foreman.rb:8: warning: previous definition of UUID_REGEXP was here
Oct 24 14:06:26 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:06:26.120Z 14667 TID-6hbxf INFO: Booting Sidekiq 5.2.7 with redis options {:url=>"redis://localhost:6379/0", :id=>"Sidekiq-server
Oct 24 14:06:26 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:06:26.123Z 14667 TID-6hbxf INFO: Running in ruby 2.5.5p157 (2019-03-15 revision 67260) [x86_64-linux]
Oct 24 14:06:26 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:06:26.124Z 14667 TID-6hbxf INFO: See LICENSE and the LGPL-3.0 for licensing details.
Oct 24 14:06:26 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:06:26.124Z 14667 TID-6hbxf INFO: Upgrade to Sidekiq Pro for more features and support: http://sidekiq.org
Oct 24 14:06:26 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:06:26.126Z 14667 TID-6hbxf INFO: Starting processing, hit Ctrl-C to stop
Oct 24 14:06:26 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:06:26.127Z 14667 TID-27o9cf INFO: Cleaning working queues
Oct 24 14:07:27 centos7-katello-nightly.war.example.com systemd[1]: dynflow-sidekiq@orchestrator.service start operation timed out. Terminating.
Oct 24 14:07:27 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:07:27.067Z 14667 TID-6hbxf INFO: Shutting down
Oct 24 14:07:27 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:07:27.067Z 14667 TID-6hbxf INFO: Terminating quiet workers
Oct 24 14:07:27 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:07:27.068Z 14667 TID-27o7kj INFO: Scheduler exiting...

Further, the current design leads to unclear logging from a system perspective in my opinion:

Oct 24 14:07:27 centos7-katello-nightly.war.example.com scl[14647]: 2019-10-24T14:07:27.068Z 14667 TID-27o7kj INFO: Scheduler exiting...

Note that the service listed is scl which is entirely unclear (especially if sending logs to a service) what this is logging changes for. This scl part is derived from the fact it is running scl enable. I am also noticing the foreman service we built suffers from this same issue.

ekohl · 2019-10-24T14:18:52Z

I'm looking at https://github.com/mperham/sidekiq/blob/master/examples/systemd/sidekiq.service and it has some neat tricks:
Reloading:

# use `systemctl reload sidekiq` to send the quiet signal to Sidekiq
# at the start of your deploy process.
ExecReload=/usr/bin/kill -TSTP $MAINPID

Memory fragmentation fix:

# Greatly reduce Ruby memory fragmentation and heap usage
# https://www.mikeperham.com/2018/04/25/taming-rails-memory-bloat/
Environment=MALLOC_ARENA_MAX=2

I'm wondering if the type=forking that's currently used is incorrect and should be type=simple. Does sidekiq fork itself?

tbrisker · 2019-10-24T14:30:32Z

considering that this is experimental in 1.24, perhaps we can merge if it works and improve afterwards if needed?

ehelms · 2019-10-24T14:48:12Z

@tbrisker That was my plan. My comment was to indicate I did not get it to work, and to identify some issues that if we don't fix now, are captured in issues that block it going from "beta" to "GA".

ezr-ondrej · 2019-10-24T16:08:49Z

I am trying to address the comments in theforeman/foreman#7127 - not to forget them.

adamruzicka · 2019-10-24T16:09:25Z

~~Addressing the start timeout (it indeed was simple vs forking) and syslog identifier issues in theforeman/foreman#7128~~

@ezr-ondrej beat me to it :)

tbrisker · 2019-10-25T09:38:31Z

what's the status here @ehelms @ekohl ?

ehelms · 2019-10-25T11:59:03Z

@tbrisker I wanted to see this in action to know this packaging change plus the code works if someone installs it and then manually configures Redis. I have not reached that point yet, working through it.

ekohl · 2019-10-25T12:13:25Z

Should we add a %exclude commit to rpm/develop in the mean time to get nightly to build again?

ehelms · 2019-10-28T12:31:24Z

[test rpm]

theforeman-bot added Needs testing Not yet reviewed RPM labels Oct 15, 2019

ekohl requested changes Oct 16, 2019

View reviewed changes

theforeman-bot added Waiting on contributor Needs re-review and removed Not yet reviewed Waiting on contributor labels Oct 16, 2019

ezr-ondrej force-pushed the dynflow-sidekiq branch from 3e6b14c to da5ca60 Compare October 16, 2019 23:01

ehelms reviewed Oct 18, 2019

View reviewed changes

packages/foreman/foreman/foreman.spec Outdated Show resolved Hide resolved

ehelms reviewed Oct 18, 2019

View reviewed changes

ezr-ondrej force-pushed the dynflow-sidekiq branch 3 times, most recently from d4a5d4f to e996cf7 Compare October 18, 2019 09:45

adamruzicka reviewed Oct 18, 2019

View reviewed changes

packages/foreman/foreman/foreman.spec Outdated Show resolved Hide resolved

ezr-ondrej force-pushed the dynflow-sidekiq branch from e996cf7 to 0e64cd8 Compare October 20, 2019 12:07

ekohl reviewed Oct 22, 2019

View reviewed changes

ezr-ondrej force-pushed the dynflow-sidekiq branch 2 times, most recently from 7a6eef5 to 72fb13d Compare October 22, 2019 14:38

ezr-ondrej force-pushed the dynflow-sidekiq branch from 72fb13d to d397dca Compare October 22, 2019 15:23

ekohl reviewed Oct 22, 2019

View reviewed changes

ezr-ondrej force-pushed the dynflow-sidekiq branch from b76c315 to 42cab18 Compare October 22, 2019 15:43

ezr-ondrej force-pushed the dynflow-sidekiq branch 3 times, most recently from 535d6c1 to 8989c1f Compare October 22, 2019 15:52

This was referenced Oct 22, 2019

Fixes #28068 - foreman dynflow-sidekiq service package #4220

Closed

Fixes #28068 - foreman dynflow-sidekiq service package #4221

Closed

ezr-ondrej force-pushed the dynflow-sidekiq branch 2 times, most recently from 39dd0ec to 74a32ec Compare October 23, 2019 16:15

ezr-ondrej added 2 commits October 24, 2019 09:25

Fixes #28065 - package dynflow sidekiq service files

f228cd8

Move the install to main install

08040e4

ezr-ondrej force-pushed the dynflow-sidekiq branch from 74a32ec to 08040e4 Compare October 24, 2019 07:25

ekohl approved these changes Oct 24, 2019

View reviewed changes

theforeman-bot removed the Needs re-review label Oct 24, 2019

ezr-ondrej mentioned this pull request Oct 24, 2019

Fixes #28126 - improve dynflow sidekiq service theforeman/foreman#7127

Merged

ehelms merged commit e2a302f into theforeman:rpm/develop Oct 28, 2019

ezr-ondrej deleted the dynflow-sidekiq branch July 22, 2020 12:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #28065 - package dynflow sidekiq service files #4194

Fixes #28065 - package dynflow sidekiq service files #4194

ezr-ondrej commented Oct 15, 2019 •

edited by ekohl

Loading

ekohl left a comment

ehelms Oct 18, 2019

ezr-ondrej Oct 18, 2019

ehelms Oct 18, 2019

ezr-ondrej Oct 22, 2019

ekohl Oct 22, 2019

ekohl Oct 22, 2019

ezr-ondrej Oct 22, 2019

ekohl Oct 22, 2019

ekohl commented Oct 22, 2019

ezr-ondrej commented Oct 22, 2019

ekohl left a comment

ezr-ondrej commented Oct 22, 2019

ekohl commented Oct 22, 2019

ehelms commented Oct 22, 2019

ezr-ondrej commented Oct 22, 2019

tbrisker commented Oct 23, 2019

tbrisker commented Oct 24, 2019

ezr-ondrej commented Oct 24, 2019

ekohl commented Oct 24, 2019

ezr-ondrej commented Oct 24, 2019

ekohl left a comment

ekohl Oct 24, 2019

ezr-ondrej Oct 24, 2019

ehelms commented Oct 24, 2019

ekohl commented Oct 24, 2019

tbrisker commented Oct 24, 2019

ehelms commented Oct 24, 2019

ezr-ondrej commented Oct 24, 2019

adamruzicka commented Oct 24, 2019 •

edited

Loading

tbrisker commented Oct 25, 2019

ehelms commented Oct 25, 2019

ekohl commented Oct 25, 2019

ehelms commented Oct 28, 2019

Fixes #28065 - package dynflow sidekiq service files #4194

Fixes #28065 - package dynflow sidekiq service files #4194

Conversation

ezr-ondrej commented Oct 15, 2019 • edited by ekohl Loading

ekohl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekohl commented Oct 22, 2019

ezr-ondrej commented Oct 22, 2019

ekohl left a comment

Choose a reason for hiding this comment

ezr-ondrej commented Oct 22, 2019

ekohl commented Oct 22, 2019

ehelms commented Oct 22, 2019

ezr-ondrej commented Oct 22, 2019

tbrisker commented Oct 23, 2019

tbrisker commented Oct 24, 2019

ezr-ondrej commented Oct 24, 2019

ekohl commented Oct 24, 2019

ezr-ondrej commented Oct 24, 2019

ekohl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehelms commented Oct 24, 2019

ekohl commented Oct 24, 2019

tbrisker commented Oct 24, 2019

ehelms commented Oct 24, 2019

ezr-ondrej commented Oct 24, 2019

adamruzicka commented Oct 24, 2019 • edited Loading

tbrisker commented Oct 25, 2019

ehelms commented Oct 25, 2019

ekohl commented Oct 25, 2019

ehelms commented Oct 28, 2019

ezr-ondrej commented Oct 15, 2019 •

edited by ekohl

Loading

adamruzicka commented Oct 24, 2019 •

edited

Loading